Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

redfish/drive: expose oem data and actions #366

Merged
merged 8 commits into from
Oct 15, 2024
Merged

Conversation

Matt1360
Copy link
Contributor

@Matt1360 Matt1360 commented Oct 9, 2024

We have some nodes that don't follow the typical indicator light flow that Update() would handle. Instead, we have to hit a linked reference (with a variable key, sigh..) from the OEM section.

I'm hoping to expose the Oem field in both the Drive and the Drive.Actions (while also exposing the Actions) so that the user can follow that chain when necessary. This allows us to get the right link to where we need to send the POST or PATCH (also variable, heh) to enable lights for our datacenter folks.

Tests updated to not lose secure erase functionality.

Copy link
Owner

@stmcginnis stmcginnis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be better addressed by adding the necessary OEM objects for this so you can access the OEM-specific properties and methods using the matching type for the hardware.

@Matt1360
Copy link
Contributor Author

Matt1360 commented Oct 9, 2024

👍 I'll take a look at adding it over there. For SMC, should I follow the other pending change and use oem/supermicro, or should it be abbreviated as oem/smc?

@stmcginnis
Copy link
Owner

Hmm, good question. I don't have a strong opinion either way really. I think most folks who need it won't have any confusion equating smc to being for SuperMicro, so I'm fine with whatever you feel like doing.

Thanks!

@Matt1360
Copy link
Contributor Author

Matt1360 commented Oct 9, 2024

I've added a commit that introduces the oem/smc package, making use of the OEM fields exposed in the first commit, along with a simple test. Is this what you had in mind? I kept the first commit in order to still expose the OEM fields in both the Drive and the DriveActions structs.

Copy link
Owner

@stmcginnis stmcginnis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start. A few issue with consistency with how we do things in Gofish, so just a few minor changes needed.

Thanks!

oem/smc/drive.go Outdated Show resolved Hide resolved
oem/smc/drive.go Outdated Show resolved Hide resolved
oem/smc/drive.go Outdated Show resolved Hide resolved
redfish/drive.go Outdated Show resolved Hide resolved
Copy link
Owner

@stmcginnis stmcginnis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really looking great. Love the way you are handling the changing action name so the library user doesn't need to be concerned about it.

Only thing I think is to just expose the new properties directly rather than getting them through an accessor.

oem/smc/drive.go Outdated Show resolved Hide resolved
oem/smc/drive.go Outdated Show resolved Hide resolved
oem/smc/drive.go Show resolved Hide resolved
oem/smc/drive.go Outdated Show resolved Hide resolved
Signed-off-by: Matt Vandermeulen <[email protected]>
@stmcginnis stmcginnis merged commit 4b8c27e into stmcginnis:main Oct 15, 2024
2 checks passed
@stmcginnis
Copy link
Owner

Nice work - thanks a lot!

@Matt1360
Copy link
Contributor Author

No problem! Do you have a sense for when the next release will be cut? Deciding whether to use my fork for now, or wait a bit and bump the version.

@stmcginnis
Copy link
Owner

You can use go get github.com/stmcginnis/gofish@4b8c27ed6527ba5cd775d82feb2f58198a33e3a2 to use the current HEAD. That's what I usually recommend.

Otherwise, I'm probably going to add a few more Supermicro OEM things since I now have access to the hardware, then tag a release once I get at least some of the main objects in place. So I guess whatever is more convenient for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants